Skip to content

ref(feedback): move userreport ingest and endpoints to feedback module #95392

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Open
wants to merge 11 commits into
base: master
Choose a base branch
from
Open

ref(feedback): move userreport ingest and endpoints to feedback module #95392

wants to merge 11 commits into from

Conversation

aliu39
Copy link
Member

@aliu39 aliu39 commented Jul 11, 2025 •
edited
Loading

User reports are the same product as feedback and owned by replay team. I'd like to move the code here for organizational purposes. Same goes for the feedback summary endpoint. This PR only moves things around, no implementation changes.

Ref REPLAY-513: [PR Tracker] refactor backend user feedback code

I plan to follow up with quality improvements to the ingest functions, making them more composable, so that module's subject to change.

@aliu39 aliu39 requested review from a team as code owners July 11, 2025 23:12
@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Jul 11, 2025
aliu39
aliu39 commented Jul 11, 2025
View reviewed changes
src/sentry/feedback/usecases/ingest/save_event_feedback.py
@@ -32,6 +35,9 @@ def save_feedback_event(event_data: Mapping[str, Any], project_id: int):

try:
# Shim to UserReport
# TODO: this logic should be extracted to a shim_to_userreport function
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doing in the next follow up

cursor[bot]

This comment was marked as outdated.

Sign in to view

@codecov Codecov
Copy link

codecov bot commented Jul 11, 2025 •
edited
Loading

❌ 1 Tests Failed:

Tests completed Failed Passed Skipped
27192 1 27191 226
View the top 1 failed test(s) by shortest run time 
tests.sentry.llm.test_openai::test_complete_prompt
Stack Traces | 0.071s run time 
#x1B[1m#x1B[.../sentry/llm/test_openai.py#x1B[0m:38: in test_complete_prompt
    assert res == ""
#x1B[1m#x1B[31mE   AssertionError: assert 'not spam' == ''#x1B[0m
#x1B[1m#x1B[31mE     #x1B[0m
#x1B[1m#x1B[31mE     + not spam#x1B[0m

To view more test analytics, go to the Test Analytics Dashboard
📋 Got 3 mins? Take this short survey to help us improve Test Analytics.

@aliu39
Copy link
Member Author

aliu39 commented Jul 11, 2025

Re: cursor - yes this is a metrics change I intentionally bundled in. We don't have a dashboard widget or monitor on this metric so going to rename it to match the other metrics in the file

@aliu39
Copy link
Member Author

aliu39 commented Jul 12, 2025

todo in follow-up: make a way to share mock_produce_occurrence patching logic, so paths don't have to be changed in so many test files every time we move or rename things.

@aliu39 aliu39 mentioned this pull request Jul 15, 2025
Merged
aliu39 added a commit that referenced this pull request Jul 15, 2025
@aliu39
fix(feedback): fix patching flakes with test_openai.py (#95577)
0cf8d97 
Discovered in #95392 the
monkeypatching of `OpenAI` class in `test_create_feedback.py` can affect
`test_openai.py`, I'm guessing when they are ran in parallel. I haven't
seen this flake anywhere else, but don't believe it's related to the
changes in #95392. Using the patch context manager should fix this.

"not spam" return value bleeds over to the openai test:
<img width="806" height="476" alt="Screenshot 2025-07-15 at 10 51 22 AM"
src="https://github.com/user-attachments/assets/d900d7d1-9138-422f-b354-ac94fad9967a"
/>

Ref [REPLAY-513: [PR Tracker] refactor backend user feedback
code](https://linear.app/getsentry/issue/REPLAY-513/pr-tracker-refactor-backend-user-feedback-code)
cursor[bot]
cursor bot reviewed Jul 15, 2025
View reviewed changes
Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bug: Feedback Saving Fails on Missing Environment

The save_event_feedback function attempts to fetch the Environment row using Environment.objects.get(...). If the referenced environment (or default "production") does not exist for the project (environments are lazily created), Environment.objects.get() raises Environment.DoesNotExist. This unhandled exception crashes the save_event_feedback task, leading to lost feedback. This is a regression from the previous implementation, which used get_or_create or similar logic to handle missing environments gracefully.

src/sentry/feedback/usecases/ingest/save_event_feedback.py#L43-L50

sentry/src/sentry/feedback/usecases/ingest/save_event_feedback.py

Lines 43 to 50 in 0d7be65

if associated_event_id:
project = Project.objects.get_from_cache(id=project_id)
environment = Environment.objects.get(
organization_id=project.organization_id,
name=fixed_event_data.get("environment", "production"),
)
timestamp = fixed_event_data["timestamp"]

Fix in Cursor • Fix in Web

Was this report helpful? Give feedback by reacting with 👍 or 👎

@aliu39
Copy link
Member Author

aliu39 commented Jul 15, 2025

Re: @cursoragent the exception is caught at the bottom of the fx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cursor cursor[bot] cursor[bot] left review comments

@nora-shap nora-shap nora-shap approved these changes

Assignees
No one assigned
Labels
Scope: Backend Automatically applied to PRs that change backend components
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@aliu39 @nora-shap